-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Fix #3185 #3186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #3185 #3186
Conversation
| this.keys.add('updatedAt'); | ||
| case 'keys': { | ||
| const keys = restOptions.keys.split(',').concat(AlwaysSelectedKeys); | ||
| this.keys = Array.from(new Set(keys)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| this.keys.add('objectId'); | ||
| this.keys.add('createdAt'); | ||
| this.keys.add('updatedAt'); | ||
| case 'keys': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm. { in case, new one on me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah to create a scope: http://eslint.org/docs/rules/no-case-declarations
| done(); | ||
| }) | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solid test.
* Adds tests that reproduce the issue * Use values from keys to force include when needed
When using
select() (i.e.: keys)with nested keysa.bparse-server doesn't include by defaultawhich make the call fail (unless you haveinclude('a')specified).This PR addresses that issue and will simplify the behaviour (and code for the clients).
For ex, calling select
a.b.cwill not include full objectsaanda.bbut only the necessary path to resolve correctly thekeysclause.Fixes #3185